Skip to content

Conversation

zeapo
Copy link
Contributor

@zeapo zeapo commented Aug 28, 2025

Since commit d2b8e74 an NPE was introduced on the filters. If no filter is provided, the views other than targets raise NPE.

I reverted the changes on the impacted lines to a proper Optional check.

If any of you understands the change and why relying on null here, I'll be glad to update the code to match the desired new behavior.

Comment on lines 119 to 126
.map(e -> {
.filter(e -> {
if (e.getValue() instanceof Optional<?> opt) {
e.setValue(opt.orElse(null));
return opt.isPresent();
} else {
return e.getValue() != null;
}
return e;
})
.filter(e -> !ObjectUtils.isEmpty(e))
.filter(e -> !(e.getValue() instanceof Collection<?> coll && coll.isEmpty()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this changes because Sonar was complaining about complexity.
I need to test it more but I think that the fix could be just:

                 .stream()
-                .map(e -> {
-                    if (e.getValue() instanceof Optional<?> opt) {
-                        e.setValue(opt.orElse(null));
-                    }
-                    return e;
-                })
-                .filter(e -> !ObjectUtils.isEmpty(e))
+                .filter(e -> !ObjectUtils.isEmpty(e.getValue()))
                 .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, your fix should work

Copy link

sonarqubecloud bot commented Sep 1, 2025

@avgustinmm avgustinmm merged commit f2e6344 into eclipse-hawkbit:master Sep 9, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants